Allow namespace scoped operator #719
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
February 27, 2024 15:06
spilchen
changed the title
roypaulin
reviewed
Feb 28, 2024
roypaulin
reviewed
Feb 28, 2024
pkg/vadmin/at.go
Outdated
| @@ -45,11 +46,11 @@ func (a *Admintools) execAdmintools(ctx context.Context, initiatorPod types.Name | |||
| // Dump relevant contents of the admintools.conf before and after the | |||
| // admintools calls. We do this for PD purposes to see what changes occurred | |||
| // in the file. | |||
| if a.DevMode { | |||
| if opcfg.GetIsDebugLoggingEnabled() { | |||
There was a problem hiding this comment.
We were doing this in dev mode so why in debug mode now?
There was a problem hiding this comment.
We used devMode as a proxy for extra debugging. I think its clearer if we use the log level. I should have done this initially.
|
|
||
| # 21. Change change ClusterRoles/ClusterRoleBindings for the manager to be | ||
| # Roles/RoleBindings if the operator is scoped to a single namespace. | ||
| for f in $TEMPLATE_DIR/verticadb-operator-manager-clusterrolebinding-crb.yaml \ |
There was a problem hiding this comment.
Where do you check if the operator is namespace-scoped?
There was a problem hiding this comment.
There are three places:
- in helm-charts/verticadb-operator/templates/_helpers.tpl, we check to know if a ClusterRole/ClusterRoleBinding should really be a Role/RoleBinding. See the "vdb-op.roleKind" and "vdb-op.roleBindingKing" defines.
- in PodSecurityReconciler, with a namespace scoped operator we don't have permissions to read the annotations of a namespace anymore.
- in pkg/opcfg/config, we use the scope to know what namespace the operator needs to watch
roypaulin
approved these changes
Feb 28, 2024
spilchen
pushed a commit
that referenced
this pull request
Mar 1, 2024
The helm chart parameter `prometheus.expose` controls if the Prometheus metrics in the operator are exposed. The old default was `ExposeWithProxyAuth`. However, this required a ClusterRole/ClusterRoleBinding to be created. This can be onerous, especially in environments where creation of those is prohibited except for cluster admins. The new default is `Disable` to make it easier to deploy in a more restricted environment. This also adds back logging helm chart parameters that were removed in #719. Instead, those parameters have been deprecated and will be removed in a future version.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change allows for deploying the operator at the namespace scope, only for Helm deployments. When deployed this way, the operator will watch only custom resources within its deployment namespace. Multiple namespace deployments are possible.
By default, the webhook is deployed with the operator. However, with multiple namespace scoped deployments in a cluster, the webhook can only be deployed once.
There are two ways to handle this:
webhook.enableto false for any subsequent operator deploymentcontrollers.enableset to false) and deploy each operator with the webhook disabled (webhook.enableset to false)A new Helm chart parameter,
controllers.scope, has been added to control the scope of the operator, with valid values of "cluster" or "namespace" and a default of "cluster."The e2e-leg-2 has been converted to deploy the operator with namespace scope.
All command-line options used by the operator have been replaced with environment variables read from a ConfigMap. This change was made when adding the namespace scope and controllers disable options, as this method is more extensible.
This change removes most of the logging Helm chart parameters, leaving only
logging.level. Those using the logging Helm chart parameters can set appropriate entries in the ConfigMap to retain the old behavior.Finally, the 'make run' option has been removed. The operator can now only be run by running a container within Kubernetes, as it was too difficult to set up a ConfigMap and environment variables to run the operator.